Skip to content

feat(mcp): delete_records deletes many records of one object in one confirmed call - #31024

Draft
objectstack-fleet[bot] wants to merge 10 commits into
mainfrom
claude/project-thread-nhupf1
Draft

objectstack-fleet[bot] wants to merge 10 commits into
mainfrom
claude/project-thread-nhupf1

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Tesla · project thread

Clause-②: yes (widening)
Metadata-protocol: none

Fixes #30891

维护者速读

  • 改了什么:MCP 新增 delete_records 工具,按筛选条件或 id 列表,一次删除同一对象的多条记录。
  • 为什么改:AI 清理一批记录时只能逐条调 delete_record、逐条确认;现在先看一次预览(共几条、样例 id),人点头后一次确认删除。
  • 风险与回滚:只新增一个工具,现有工具不变。每条记录走 delete_record 的同一删除路径(权限、行级安全、包的锁定规则),管理员也绕不过;每次最多 200 条(部署的读取页上限更小时按它)。遇到故障(驱动错误、代码钩子崩溃)立即停止并报告已删条数。OAuth 令牌需同时有 data:read 和 data:write。回滚即 revert 本 PR。
  • 先看:packages/mcp/src/mcp-http-tools.ts 里 delete_records 的注册段和 deleteEach。
  • 两分钟验证:pnpm --filter @objectstack/mcp exec vitest run src/mcp-delete-records.test.ts(27 条);真实栈上的锁定规则见 packages/qa/dogfood/test/mcp-delete-records.dogfood.test.ts。
  • 需要你做的:无需决定。注意一点:预览无法预先说出哪些记录会被锁定规则拒绝(引擎没有删除试运行),确认后的回答按原因分组列出被拒的记录和 id。

What changes

One new tool on both MCP transports (HTTP /api/v1/mcp and the stdio server, which share registerObjectTools): delete_records. Over OAuth it is offered only to a token that carries both data:read and data:write.

Request

{
  "objectName": "crm_task",
  // exactly one of:
  "where": { "status": "closed" },          // a filter, as query_records takes it; {} = every record you can see
  "recordIds": ["t1", "t2"],                // 1..200 ids
  "confirm": true                           // optional boolean; only true deletes
}

Unconfirmed call = the preview. It reads the selection as the caller and deletes nothing. It answers delete_record's refusal from #30861 (same member, code, status, envelope keys and details keys; details.actionName is delete_records) with a preview beside error:

{
  "error": { "code": "ACTION_CONFIRMATION_REQUIRED", "message": "delete_records on 'crm_task' carries no confirmation — nothing was deleted. The selection matches 4 record(s); …", "status": 428,
             "details": { "actionName": "delete_records", "objectName": "crm_task", "confirmationMember": "confirm" } },
  "preview": { "object": "crm_task", "matched": 4, "wouldAttempt": 4, "cap": 200,
               "sampleRecordIds": ["…"], "notFound": ["…"] /* id list only */, "note": "Nothing was deleted. …" }
}

Confirmed call. Each selected record is deleted through the bridge's remove, the member delete_record deletes with, one at a time, under the caller's resolved context. Refused records stay and are grouped by code, status and reason:

{
  "object": "crm_task", "matched": 4, "attempted": 4, "deleted": 2, "refused": 2,
  "refusals": [ { "code": "RECORD_LOCKED", "status": 409, "reason": "<the rule's own sentence>", "count": 2, "recordIds": ["…", "…"] } ],
  "remaining": 2, "cap": 200,
  "next": "…"  // only when more than 200 matched
}
  • The cap is 200 per call. That is the bound the platform's other many-row delete door already uses: REST POST /data/:object/deleteMany refuses more ids than batch.maxBatchSize, default 200. Like that route, this tool deletes one row at a time, so the cap also bounds how long one call runs. A filter is read in id order at the cap, never one row past it, and the bridge's aggregate count says whether more match. When more than 200 match, remaining and next say how many are left and that the same call continues. When refused rows sit inside the first 200, next names {"id": {"$nin": [...]}} to move past them. A bridge without aggregate keeps a cap+1 probe and answers matchedMoreThan: 200, not a guess.
  • The cap follows the deployment's read window down. The data door refuses (never clamps) a read over OS_DATA_MAX_PAGE_SIZE or past OS_DATA_QUERY_ROW_BUDGET, and the caller has no limit to lower. So on a deployment that sets either under 200, one call runs at that size (deleteRecordsCap), and the answer's cap names it. An id list longer than that cap is refused before anything is read.
  • A fault stops the call; a rule's refusal does not. Each row's throw is judged with the REST /data door's own classifier (mapDataError, @objectstack/types), so the two doors cannot disagree on a row. A 4xx verdict is a refusal and is grouped: a coded rule refusal under the code and message delete_record answers, a sandboxed hook body's uncoded throw as 400 HOOK_ABORTED with the body's sentence. A 5xx verdict is a fault: a code hook's bare throw new Error, a lost connection, a driver error, and a driver's own string code (ECONNREFUSED, a SQL state, SQLITE_BUSY), which the door answers 500 too. The first fault ends the call: isError: true, an error naming the fault, the record it stopped at, how many were deleted (they stay deleted) and how many were not attempted, and a progress member with the call's answer so far (stoppedAt, notAttempted, no next).
  • No bypass, for an administrator either (ruling 6687860173, 「不绕过」). The tool has no delete path of its own. It calls the same remove as delete_record, with the context the principal resolver answered, so the engine judges each row's beforeDelete hooks, restricted relations, permissions and RLS as it does for a one-row delete.
  • details withheld. A refusal is read with the one-row tool's reader (thrownRefusalEnvelope: code, message, status), so a refusal's details (e.g. a permission refusal's positions) stays off the wire, as on every other tool.
  • ADR-0090 D10. A narrowed delegated read is stated on the preview and on the answer, as query_records does.

Also: the SKILL.md tool list and the package README name the tool. content/docs/ai/connect-mcp.mdx gets the table row and a security-model bullet. In @objectstack/spec, only doc comments change: AIActionConfirmation's docblock names delete_records as the third door, and the error-code ledger's ACTION_CONFIRMATION_REQUIRED rows name both delete tools. There is one changeset, patch level, for @objectstack/mcp, @objectstack/spec and @object-ui/console.

Files

  • packages/mcp/src/mcp-http-tools.ts: the tool, resolveDeleteSelection, deleteEach, rowRefusalOf, deleteRecordsCap.
  • packages/mcp/src/skill-md.ts, packages/mcp/README.md, content/docs/ai/connect-mcp.mdx: teaching.
  • packages/spec/src/contracts/ai-service.ts, packages/spec/src/api/error-code-ledger.zod.ts: doc comments only.
  • ui/apps/console/src/pages/developer/objectstack-skill.ts and one line of objectstack-skill.test.ts: the console's "Download skill" text, a byte-for-byte mirror of skill-md.ts under fix(console): the skill download teaches delete_record confirm: true and mirrors the served SKILL.md #31015's drift pin, carries the same delete_records bullet. This is outside the card's ui/** fence, under seat claim amendments 6692914335 and 6692945782.
  • Tests: mcp-delete-records.test.ts (new), the dogfood file (new), and the tool-surface pins listed under Tests.
  • scripts/engine-double-contract.pinned.json: the new engine double's ledger row.
  • .changeset/30891-mcp-delete-records.md.

Tests

  • packages/mcp/src/mcp-delete-records.test.ts (27 tests, real stdio bridge on an engine double whose delete refuses locked rows with RECORD_LOCKED/409, and other users' rows with PERMISSION_DENIED/403 carrying details):
    • advertisement: destructive hint, optional boolean confirm, recordIds.maxItems 200;
    • preview: 428 refusal plus preview (count, sample), engine.delete never called, all rows intact; false, "true" and 1 are refused too;
    • the refusal has delete_record's code, status, envelope keys and details keys;
    • confirmed: allowed rows deleted, refused rows grouped with ids, details withheld, one engine delete per row;
    • every engine read and delete carries the caller's context object unchanged (no isSystem, no copy);
    • id list: duplicates collapse, notFound;
    • cap: 205 rows, the first call deletes 200 with remaining: 5 and next, the second finishes; over the cap with refused rows, next names $nin;
    • no aggregate: matchedMoreThan: 200;
    • a stale roll-up counts as deleted; success: false lands in notRemoved;
    • selection xor; an id list over 200 is refused at the input;
    • the read window, through the platform's own planQueryReadWindow in front of the bridge: a page size equal to the cap reads at 200 and counts 205 (a lit control shows the window refusing 201); exactly 200 matching has no next; a decimal-string count is read; a page size or row budget of 50 lowers the cap to 50; an id list longer than a lowered cap is refused before any read;
    • faults: an uncoded throw, a coded 5xx and a driver's ECONNREFUSED each stop the call at that row (isError, the record, the deleted count, progress, later rows never asked); a sandboxed body's uncoded refusal is grouped as HOOK_ABORTED/400 and the call goes on;
    • the long-lived stdio server (InMemoryTransport) behaves the same.
  • packages/qa/dogfood/test/mcp-delete-records.dogfood.test.ts: a real stack with a package beforeDelete lock in HotCRM's shape, called by the administrator over the live /api/v1/mcp. It covers:
    • the preview (REST still reads every row);
    • the REST DELETE control (409 for the same admin);
    • the confirmed call: unlocked rows gone, locked rows one RECORD_LOCKED/409 group with the hook's own sentence, and still readable;
    • the cap on 203 real rows with the runtime's real aggregate count, on a host whose OS_DATA_MAX_PAGE_SIZE is 200 (lit control: REST ?limit=201 answers 400 MAX_RECORDS_EXCEEDED);
    • a host whose page size is 2: the call runs at 2 and says so, and a 3-id list is refused;
    • a sandboxed hook body's refusal grouped as HOOK_ABORTED/400 while the walk goes on, then a code hook's bare throw new Error stopping it, with REST DELETE as the control (400 and 500 on the same rows).
  • Tool-surface pins learn the thirteenth tool: the scopes, authoring-scope, HTTP and stdio tool lists, unknown-argument keys, the coded-refusal sites, the SKILL.md list and the stored-metadata-body read-site census. The scopes pin now has delete_records only under both data scopes, and a data:write-only tools/call delete_records refused with no query, aggregate or remove reaching the bridge.
  • The console's SKILL.md drift pin (fix(console): the skill download teaches delete_record confirm: true and mirrors the served SKILL.md #31015) and its tool-name list include delete_records.

Acceptance notes

  • Card premise, partly falsified: the preview cannot name per-record refusals. The engine has no delete dry run (BatchOptions.validateOnly was retired for promising one). Running the delete in a rolled-back transaction would still fire afterDelete hooks, which do not wait for a commit. So the preview gives the count and a sample and says the rules are judged when the delete runs. The confirmed call's answer carries the grouped refusals with ids.
  • Groups key on code + status + message, not on a rule name. The engine attaches no hook or rule name to a refusal, so two rules that throw the same code, status and sentence share a group.
  • Both data scopes (seat decision on review round 1). The preview reads ids and a count, a list-class read, so a token granted data:write without data:read is not offered the tool. delete_record stays under data:write alone.
  • The cap follows the read window down: an extension beyond the review's literal ask. The review asked to read at the cap instead of cap+1, which fixes a window of exactly 200. A window under 200 would still refuse every filter selection, with a message naming a limit the caller never sent, so the cap takes the window's size instead.
  • Fault vs refusal follows the data door, not "coded vs uncoded". The review's rule was "an uncoded throw is a fault". Measured on a booted stack, a sandboxed hook body's deliberate refusal also arrives uncoded (REST answers it 400 HOOK_ABORTED), and a driver's errors often carry string codes (ECONNREFUSED, 57P01, SQLITE_BUSY; REST answers them 500). So the tool asks the REST door's classifier and stops on its 5xx verdicts.
  • An object whose apiMethods exclude list cannot use delete_records. The selection is a list-class read, so the exposure gate refuses it. delete_record still works there.
  • Out of scope, follow-ups: a delete dry run in the engine, which would let the preview name refusals (the seat files it as its own card); skills/objectstack-ai/SKILL.md teaching delete_records (skills/** is outside this card).
  • Serial with MCP cannot read or attach files: no tool downloads an attachment (e.g. a policy PDF to analyse) and none attaches one (e.g. a photographed invoice) to a record #30892 / PR feat(mcp): read_file and create_upload_link read and attach a record's files #31009 (MCP attachments). It touches the same files: three count/prose conflicts (connect-mcp.mdx, the unknown-argument-keys test, the skill-md.ts imports), and the merged tool count is 15. Whichever lands second takes them, and must also mirror its own skill-md.ts lines into the console copy. This branch does not merge feat(mcp): read_file and create_upload_link read and attach a record's files #31009's. origin/main was merged in again at 17a02908 (fix(console): the skill download teaches delete_record confirm: true and mirrors the served SKILL.md #31015's drift pin arrived there).

Verification

Head 4e5b0f2d (origin/main merged at 17a02908):

  • pnpm --filter @objectstack/mcp test: 58 files, 725 tests passed. mcp-delete-records.test.ts has 27.
  • pnpm --filter @objectstack/mcp run typecheck (tsc --noEmit plus check:test-typecheck, debt ledger held) exited 0.
  • Dogfood: mcp-delete-records.dogfood.test.ts (7) and mcp-delete-record-confirmation.dogfood.test.ts (4) passed, 11/11, after a full ./packages/** + @objectstack/dogfood^... build. The dogfood resolves @objectstack/mcp from dist.
  • Console: pnpm --dir ui exec vitest run apps/console/src/pages/developer/objectstack-skill.test.ts, 13/13.
  • eslint over every touched .ts file (packages and the two console files) exited 0.
Ablations, round 1: four new negative pins, each mutated once through scripts/ablation-replace.mjs (WRAP mode), each red, each restored. mcp-http-tools.ts is byte-identical on 4e5b0f2d (blob d06c686d).

B1. A data:write-only token is refused. Mutation: if (canRead) { becomes if (true) {, so the tool registers under data:write alone.

     × data:write alone exposes only the write family, without delete_records
     × [#30891] delete_records with a data:write-only grant is refused WITHOUT reading or deleting
     × a data:write token can neither list nor call an authoring tool, and the registry is never touched
      Tests  3 failed | 27 passed (30)
AssertionError: expected [ [ 'query', 'task', { …(4) } ] ] to deeply equal []

B1 ran before the round's commit; its restore put the file back to the older HEAD. The round's edits were replayed and proved byte-identical (blob d06c686d, the blob B1 mutated), then committed before B2–B4.

B2. A fault stops the call. Mutation: if (door.status >= 500) return undefined; becomes if (false) return undefined;, so a 5xx verdict is grouped like a refusal.

unit:     × an uncoded throw … / × a coded 5xx … / × a driver's own string code … ends the call at that row
          Tests  3 failed | 24 passed (27)
dogfood:  × confirmed: the refusal is grouped and the walk goes on; the fault stops it, naming the row and what was deleted
          Tests  1 failed | 6 passed (7)
ablation-replace: ok restored: blob == HEAD (d06c686d20cf) and `git diff HEAD` is empty

B3. The selection is read at the cap. Mutation: const ids = await readIds(cap); becomes const ids = (await readIds(cap + 1)).slice(0, cap);, the old probe.

unit:     × a host whose maximum page size is the cap … / × exactly the cap matching … / × (both lowered-cap cases)
          Tests  4 failed | 23 passed (27)
dogfood:  × the cap, on a host whose maximum page size IS the cap … / × a host whose maximum page size is under the cap …
          Tests  2 failed | 5 passed (7)
          [data] read refused: code=MAX_RECORDS_EXCEEDED surface=list object=dlr_bulk_row rows=201 limit=200
ablation-replace: ok restored: blob == HEAD (d06c686d20cf) and `git diff HEAD` is empty

B4. The cap follows the read window down. Mutation: deleteRecordsCap returns DELETE_RECORDS_MAX_PER_CALL unconditionally.

     × a maximum page size under the cap lowers this call's cap to it … / × a query row budget … / × an id list longer than a lowered cap …
      Tests  3 failed | 24 passed (27)
AssertionError: expected 200 to be 50 // Object.is equality

The dogfood runs for B2 and B3 rebuilt @objectstack/mcp's dist inside the mutation; it was rebuilt again after the restore.

Ablations from the first round (A1–A3), on ef1973df, unchanged by this round
  • A1. The preview runs the deletes: 7 red.
  • A2. The confirmation gate removed: 7 red.
  • A3. The runtime bridge's remove hands the engine { ...ec, skipAutomations: true } (the admin-bypass shape): the dogfood admin pin red (deleted 4, refused 0); the REST control stayed green.

Gates on 4e5b0f2d:

  • dispatch-gates.mjs --commands derived 116 commands.
  • 8 artifact-roster families that name a path of this diff were added: check-published-list-mirrors.mjs, spec check:meta-url-spelling, check:authz-resolver, check:changeset-fixed, check:facade-delegation-only, check:filter-alias-parity, check:i18n-stale-fill and check:published-readme-exports.
  • All 124 ran with stdin from /dev/null, and every one exited 0.
  • dispatch-gates --ran reconciled 116 derived, 116 run, 0 NOT-MEASURED.
  • The record is server-seat-2/r2026-11-06/out/30891-gates-ran-4e5b0f2d.list in the project files.
  • The changeset: one added file; changeset status and check:changeset-fixed exited 0.
Notes on the gate runs
  • First sweep, on e223b854. check:where-matcher exited 1 on a real finding: the new test's engine double read a combinator ($and, $or) as a field name. It now refuses any $-key inside its every callback (a7989233).
  • Round 1. check:test-typecheck exited 1 on a real finding: the new scope pin read two JSON-RPC answers as unknown, growing the package's debt. They are typed (4e5b0f2d).
  • Prerequisites, not findings. Spec dist, client-react dist, the dual-build dists and pinned commit 621a4876 were built or fetched first, as in round 0.
  • The sweep ran after a full ./packages/** + @objectstack/dogfood^... build on the merged head.

…onfirmed call (WIP)

The tool selects by a filter or an id list, answers an unconfirmed call
with delete_record's confirmation refusal plus a preview (count, sample
ids), and on confirm deletes each row through the bridge's remove, so
every row meets the caller's permissions, row-level security and the
package's delete rules; refusals come back grouped by reason. 200 per call.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5
…xt and the 200 cap (WIP)

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5
…hangeset; dogfood pins (WIP)

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5
@github-actions github-actions Bot added size/xl documentation Improvements or additions to documentation tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Nov 6, 2026
@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/mcp, @objectstack/spec, touching 40 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/mcp/README.md, packages/mcp/src/skill-md.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

37 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 90523379a149e85afa1ed49286a3abaa95075c42.

⛔ 12 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 2 changed file(s) yielded no anchor (packages/mcp/README.md, packages/mcp/src/skill-md.ts) — pages documenting those are invisible to this run
  • 1 cross-cutting symbol(s) contributed no route anchor: recordId (4 routes)
  • 9 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 63 of 222 client-bound route-ledger rows — the other 159 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 159: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 104 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 176 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 90523379a149e85afa1ed49286a3abaa95075c42 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 1cef3bb598cc3a7f78c6152eaf71fb877b8eea18 — the merge of head 4e5b0f2dc8e8b3e32eda6679524596fd1057b8dd into base 90523379a149e85afa1ed49286a3abaa95075c42, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1cef3bb598cc3a7f78c6152eaf71fb877b8eea18 && git checkout 1cef3bb598cc3a7f78c6152eaf71fb877b8eea18
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 90523379a149e85afa1ed49286a3abaa95075c42 4e5b0f2dc8e8b3e32eda6679524596fd1057b8dd && git checkout -B drift-repro 90523379a149e85afa1ed49286a3abaa95075c42 && git merge --no-ff 4e5b0f2dc8e8b3e32eda6679524596fd1057b8dd

node scripts/docs-audit/affected-docs.mjs --json 90523379a149e85afa1ed49286a3abaa95075c42

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 90523379a149e85afa1ed49286a3abaa95075c42 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-tesla · session_01L9qibyvxbrLP4jm4PargoH

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: ef1973dfe1dc58bfd1bc870f2a507a3c17a0d9d6
Local-runs: none
Scope: packages/mcp/src/mcp-http-tools.ts (+406 / −8: isDeleteConfirmed, deleteConfirmationRefusal, DELETE_RECORDS_MAX_PER_CALL, resolveDeleteSelection, countSelection, deleteEach, deleteRecordsPreview, deleteRecordsRefusalMessage, deleteRecordsOutcome, the delete_records registration inside if (canWrite)), packages/mcp/src/skill-md.ts, packages/mcp/README.md, packages/mcp/src/mcp-delete-records.test.ts (new, 17), packages/qa/dogfood/test/mcp-delete-records.dogfood.test.ts (new, 4), the eight tool-surface pins (mcp-coded-refusal, mcp-http-tools.scopes, mcp-http-tools.unknown-argument-keys, mcp-server-runtime.authoring-scope, mcp-server-runtime.http, mcp-stdio-tools, skill-md, stdio-data-bridge.stored-metadata-body), packages/spec/src/contracts/ai-service.ts (docblock only), scripts/engine-double-contract.pinned.json, content/docs/ai/connect-mcp.mdx, .changeset/30891-mcp-delete-records.md (read as the ② input) + cited on origin/main (7d9f1e973e): packages/mcp/src/mcp-http-tools.ts 430–540 (errorResultFromThrown, errorResultWithCode, landedWriteResultOrError, codedErrorResult, the ACTION_CONFIRMATION_REQUIRED constant), 592–680 (thrownRefusalEnvelope, wireCodeOf, withDelegationNotice), 1067–1082 (maxLimit, canRead / canWrite, guard), 1340–1400 (query_records), 1585–1631 (delete_record: gate before the bridge, bridge.remove, landedWriteResultOrError); packages/mcp/src/stdio-data-bridge.ts 1018–1060 (query: resolvePrincipal per call, enforceApiExposure on the query verb, engine.find(object, query, { context }), the { object, records, total } shape), 1146–1168 (remove: exposure gate, findById as the caller, engine.delete(object, { where: { id }, context })), 1171–1215 (aggregate: list-class exposure gate, context on the engine call); packages/runtime/src/domains/mcp.ts 555–600 and 736–830 (the principal-bound bridge: ec and envId from the request, query / aggregate / remove each callData(..., driver, envId, ec)); packages/runtime/src/action-execution.ts 579–600 (callData: exposure gate skipped only when executionContext.isSystem, qlOpts = { context: executionContext }), 775–800 (the delete arm: ql.find then ql.delete with findOpts, i.e. the caller's context), 815–860 (the query arm: protocol.findData({ ..., context: executionContext })); packages/metadata-protocol/src/protocol-data-crud.ts 560–760 (findData's read window: planQueryReadWindow), packages/types/src/query-read-limits.ts 11–60 and 227–280 (OS_DATA_MAX_PAGE_SIZE / OS_DATA_QUERY_ROW_BUDGET: "refuse, never clamp", MAX_RECORDS_EXCEEDED); packages/objectql/src/engine-top-level.ts 88–135 (ruled #7477: after* hooks dispatch INSIDE the unit, before commit); packages/spec/src/api/batch.zod.ts 86–96 and 129 (validateOnly retired, batch.maxBatchSize default 200); packages/spec/src/api/error-code-ledger.zod.ts 1435–1455 (the @objectstack/mcp row); packages/spec/src/contracts/action-confirmation-contract.pin.test.ts 55–100 (one second emitter, @objectstack/mcp); packages/objectql/src/engine-hook-dispatch.ts (a hook's throw propagates as thrown; no rule name attached).

Inputs read: the seat's order dispatch-30891.md and FRAME.md; card #30891 body and all nine comments (triage 6685138600 and 6685223929 — item 3 split to decision card #30939; seat 4's void claim 6685253134 and park 6685265365; the blocker repair 6686810557; the ruling 6687860173 at 2026-11-06T14:58Z, option A 「不绕过(推荐)」; the release 6689577721; seat 2's claim 6689893243 naming claude/project-thread-nhupf1; the os-dev report 6692233440 at 2026-11-06T18:23Z); PR #30861's body, its 18-file list and merge c77dd17f26 at 2026-11-06T16:11Z (the confirmation contract: AI_ACTION_CONFIRMATION_MEMBER, ACTION_CONFIRMATION_REQUIRED / 428, ActionConfirmationRequiredDetails, one shared renderer codedErrorResult, the ledger's @objectstack/mcp row); PR #31024's body, its six commits over merge-base bcf8a37e08 (be641438, df996a76, 5cc50f96, e223b854, a7989233, the main merge ef1973df), the 17-file list (+1218 / −18), the net diff origin/main...ef1973df; PR #31009's state (open, draft, head 8d23d823, 31 files, its own merge-base with this head 8ae0ab9b); git merge-tree --write-tree ef1973df 8d23d823 → tree f41ec6d6, exit 1, three content conflicts (below). Check-runs on ef1973df, read at 2026-11-06T18:35Z: 62 total, 26 completed/success, 8 completed/skipped, 0 failed, 28 in_progress (Check Changeset, Governed Surface Queue Guard, Dogfood Regression Gate, PR-body guards, Spec property liveness, Lint · ESLint, Lint · PM dispatch-gates, Lint · scoped · tooling self-tests, Lint · scoped · slot lookup & corpus, Check PR Size, Check Documentation Links, Build Docs, Build UI Deps green; Build Core, Type Check · workspace, Test Core 1–8, Test UI 1–16, Type Check UI, Build & E2E UI still running). Both carriers read by eye: PR body and changeset each carry Clause-②: yes (widening) and Metadata-protocol: none as line-initial keys, one each; no Skills-docs: line, none owed (protocol line is none). Draft, mergeable: true, mergeable_state: blocked (draft), author objectstack-fleet[bot], one commit behind origin/main (#31015, the console skill mirror, touches no file of this diff).

① Derived judgments

  • The preview deletes nothing. The handler's order is guard → selection xor → resolveDeleteSelection (bridge query, and aggregate only when the filter read came back over the cap) → diagnoseDelegation probe → isDeleteConfirmed(args). The unconfirmed branch builds the refusal and the preview from resolved and returns; deleteEach is reachable only after the predicate. The bridge members an unconfirmed call touches are query, aggregate and diagnoseDelegation, all reads. Pinned: engine.delete never called and every row still present on the unconfirmed call; false / "true" / 1 refused with six rows intact; the dogfood's preview followed by five REST 200s. Ablated (A1: deleteEach run in the unconfirmed branch → 7 red, "expected vi.fn() to not be called at all, but actually been called 5 times"). TRUE.
  • An unconfirmed call deletes nothing, by the one rule feat(mcp)!: delete_record refuses a call without confirm: true #30861 landed. isDeleteConfirmed is the predicate recordDeleteConfirmationRefusal was rewritten onto (request[AI_ACTION_CONFIRMATION_MEMBER] === true, boolean only), and deleteConfirmationRefusal is the one builder of { code: ACTION_CONFIRMATION_REQUIRED, message, status: 428, details: { actionName, objectName, confirmationMember } } for both tools; delete_record's own refusal now goes through it with DELETE_RECORD_TOOL, byte-for-byte the previous envelope. No second shape: the many-row refusal is the one-row refusal with details.actionName: 'delete_records', plus a preview member BESIDE error in the text content (the error object itself is unchanged). The input declares the member as delete_record does: z.boolean().optional(), so an unconfirmed call reaches the handler and gets the coded refusal rather than a schema error; the strict input refuses "true" and 1 before the handler. Pinned: Object.keys(many) equals Object.keys(one), details keys equal, confirmationMember and objectName equal, actionName is the tool name (a lit control that the one-row half really is MCP delete_record refuses a call without confirm: true (ACTION_CONFIRMATION_REQUIRED), the run_action confirmation gate widened to deletes #30813's). Ablated (A2: the gate removed → 7 red, "expected undefined to be true" on isError). The spec pin action-confirmation-contract.pin.test.ts still holds: the ledger lists the code under @objectstack/runtime and exactly one second emitter @objectstack/mcp; this PR adds no ledger row. TRUE.
  • The confirmed call runs as the caller through the engine's own delete path. The tool holds no execution context and no engine: it calls bridge.remove(objectName, id) per id, the member delete_record calls. On the stdio bridge, remove resolves the principal per call, gates exposure on the remove verb, probes findById as the caller, then engine.delete(object, { where: { id }, context }). On the runtime bridge, remove is callData('delete', { object, id }, driver, envId, ec): the exposure gate (skipped only for isSystem, which the tool never sets), ql.find then ql.delete under findOpts carrying ec. So each row meets its own beforeDelete hooks, restricted relations, permissions and row-level security inside the engine; the tool adds no execution path and no elevated copy. Pinned: every engine read and delete carries the very object the principal resolver answered (toBe(CALLER), roles ['admin']), isSystem undefined on all five delete contexts; the dogfood's administrator (stack.signIn()) is refused by a real package beforeDelete lock (RECORD_LOCKED / 409, "a USER's delete of a closed-period row is refused; a write with no acting user passes"), the locked rows still 200 over REST, with the REST DELETE control answering the same admin 409. Ablated (A3: skipAutomations: true added to the runtime bridge's delete context → dogfood 1 red, deleted 2 → 4, refused 2 → 0, the REST control stayed green). Matches ruling 6687860173 (no administrator bypass of a package's locking rules). TRUE.
  • The selection is read as the caller, and nothing read outside it is deleted. A filter: bridge.query(objectName, { where, fields: ['id'], orderBy: [{ field: 'id', order: 'asc' }], limit: cap + 1 }); an id list: { id: { $in: listed } } with limit: listed.length (≤ 200 by the schema's max). Both bridges apply the object's exposure gate on the query verb and the engine's RLS (context on find; protocol.findData with context: executionContext). An id the caller cannot read lands in notFound ("naming no readable record"), is never attempted, and discloses no existence. queriedRows throws rather than reading an unreadable answer as "zero rows". TRUE.
  • Organization boundary. The runtime bridge is built per request from context.executionContext and context.environmentId; every member is bound to that ec and envId, and the tool only composes bridge members. The stdio bridge resolves its principal per member call. No org id, tenant id or environment is read or passed by the tool. The count (aggregate) goes through the same bridge member under the same context (stdio-data-bridge.stored-metadata-body.test.ts census now lists mcp-http-tools.ts: aggregate(objectName) as a BRIDGE read, not an engine read). TRUE.
  • Grouping key and bounds. deleteEach keys a refusal on JSON.stringify([code ?? null, status ?? null, reason]) with reason = thrownRefusalEnvelope(err)?.message ?? messageOf(err); groups sorted largest first, ties in refusal order. The card's "(rule name)" is not available: the engine dispatches a hook's throw as thrown (engine-hook-dispatch.ts attaches no hook name; a metadata-bound verdict throws new Error(verdict.message) with the declared code and status), and the dogfood's group reason is the hook's own sentence. The per-record ids are bounded by construction: attempt.length ≤ 200, every refused id is one of attempt, so all recordIds across all groups total at most 200. details of a refusal (a PERMISSION_DENIED's positions) stays off the wire — the reader is thrownRefusalEnvelope (code, message, status only), pinned by not.toContain(WITHHELD_POSITION). TRUE; see should-fix 2 for an uncoded throw.
  • The cap and the remainder. DELETE_RECORDS_MAX_PER_CALL = 200, the REST deleteMany route's batch.maxBatchSize default (confirmed in batch.zod.ts); the recordIds schema .max(200) refuses a longer list at the input (pinned, 201 ids, engine.delete not called). A filter is read cap + 1 in id order; moreThanCap is true when the read returned more rows than the cap; matched is then the bridge's aggregate count (or absent → matchedMoreThan: 200, never a guess); attempt = ids.slice(0, cap). The answer carries attempted, deleted, refused, remaining = matched − deleted (the refused, the not-removed and the ones past the cap all still match) and next naming the count past the cap and, when refused rows sit inside the first 200 of a larger selection, the $nin hint. Pinned on 205 rows (200 then 5, id order), on 202 with two locked (remaining: 4, $nin in next), on 203 without aggregate (matchedMoreThan, no remaining), and in the dogfood on 203 real rows with the runtime's real aggregate. The read-limit rule is "refuse, never clamp": a host whose OS_DATA_MAX_PAGE_SIZE or OS_DATA_QUERY_ROW_BUDGET is under 201 makes the filter read refuse with MAX_RECORDS_EXCEEDED (loud, through errorResultWithCode), so the cap arithmetic can never be silently short — but the filter path is then unusable on such a host (should-fix 1). TRUE.
  • Refusal codes in the ledger. The tool stamps one code, ACTION_CONFIRMATION_REQUIRED, listed under @objectstack/mcp since feat(mcp)!: delete_record refuses a call without confirm: true #30861; the per-row codes in the answer are relayed from the engine / hooks through wireCodeOf (the DUPLICATE_RECORD → UNIQUE_VIOLATION arm included), not emitted here. No new code, no ledger change, check:error-code-casing and check:error-status-conformance exit 0 in the dev's sweep. TRUE; the ledger row's comment names only delete_record (nit 1).
  • Tests and ablations pin each negative. Preview deletes nothing (unit + dogfood; A1); unconfirmed call deletes nothing, non-boolean members included (unit; A2); administrator still refused by a locking rule (dogfood with the REST control; A3); plus the caller-context identity pin, the details withholding pin, the selection xor pin, the input-cap pin, the no-aggregate pin, the stale roll-up / success: false pin, and the long-lived stdio server pin. A1 and A2 light the same seven tests by different mutations; acceptable since they put back different forbidden behaviours (a delete before the gate; the gate itself). A3 mutates a file this diff does not touch (the runtime bridge's delete context) — the right place, because the tool holds no context to elevate; the unit pin on context identity covers the tool's own half. The engine double refuses combinators and unmodelled operators loudly (a7989233, after check:where-matcher caught it). TRUE.
  • Declarations. Clause-②: yes (widening) — a new tool on both transports, no existing tool, input or answer changed (the delete_record refusal is rebuilt through the shared builder with identical output). Metadata-protocol: none — nothing a metadata author writes. No Skills-docs: owed. patch on both packages follows from none (AGENTS.md Post-Task Checklist, as feat(mcp)!: delete_record refuses a call without confirm: true #30861 recorded); Check Changeset green. TRUE.
  • No packages/spec key added. The spec diff is +13 / −1 in ai-service.ts: a "third gate" paragraph on AIActionConfirmation and one phrase on ActionConfirmationRequiredDetails.actionName. No type, constant, schema or export moves; check:api-surface, check:spec-docblock-symbol-anchors exit 0. Within the order's allowance. TRUE.
  • Docs match behaviour. connect-mcp.mdx: "Thirteen tools", the table row ("by a filter or an id list, at most 200 per call"), the security-model bullet (same gate, refusal is the preview, 200 per call, each row as delete_record, no caller bypasses, administrator included, grouped refusals with ids, remainder), and the tools/list comment. Each sentence has a line in the code or a pin. SKILL.md line interpolates DELETE_RECORDS_MAX_PER_CALL; README tool-list and scope lines. TRUE.
  • PR body vs diff. Every claim in "What changes" is in the diff: the request shape, the preview-as-refusal, the outcome shape, the cap and its origin, the no-bypass sentence, details withheld, ADR-0090 D10 on both answers (stated(...)), the SKILL / README / docs / spec docblock lines, one changeset. Ablations, suites and the gate sweep (111 derived + 9 roster, 120 exit 0, --ran 111 / 111 / 0 NOT-MEASURED) are the dev's readings, not re-run here (Local-runs: none); CI on the head is read above and is not finished.

② Semver level

Both carriers read Clause-②: yes (widening) and Metadata-protocol: none, line-initial, one each. patch on @objectstack/mcp (the tool) and @objectstack/spec (a doc comment) is the level none grades, the spelling #30861's accepted changeset used for the same two packages. No !, no BREAKING section, no ADR-0087 marker owed (nothing narrows: check-changeset-no-major and check-adr-0087-registration exit 0 in the sweep; Check Changeset green on the head). The changeset body states the three behaviours (preview is the refusal; each record as delete_record, no bypass; 200 per call with remaining / matchedMoreThan) and what a preview cannot say. Consistent with the code in every field.

Check-runs on ef1973df at 2026-11-06T18:35Z: 26 success / 8 skipped / 0 failed / 28 in progress. The seat re-reads before landing; a red Test Core, Type Check or Build Core lane reopens this record. Draft; no governed path (skills/**, ui/**, docs/adr/**, .claude/** untouched); 1,236 changed lines, under the 3,000 line human-merge threshold.

③ Boundary flags

  • The dev's deviation (1): the preview cannot name per-record refusals. Judged against the engine: BatchOptions.validateOnly is retired (BatchOptions.validateOnly 声明了 dry-run 但从不实现 —— "预演"会真实落库(PD #10) #4052, batch.zod.ts 86–96, "promised a dry run while…"); after* hooks dispatch INSIDE the unit before commit by the afterDelete fires inside a rollback-able transaction, so a rolled-back cascade leaves a hook that fired for a row that still exists #7477 ruling (engine-top-level.ts 88–135), so a delete run and rolled back would still fire afterDelete effects that leave the engine; the engine exposes no door that evaluates beforeDelete without deleting. So the preview answering count, sample and "judged at delete time" is the only true preview this engine can give, and the grouped refusals with ids arrive on the confirmed answer. Honest: the deviation is stated in the PR body's first acceptance note, in the 维护者速读 "需要你做的" line, the changeset, the docs bullet, the tool description, the preview's own note and the code comment; the preview claims nothing it cannot do. Against the card's acceptance: the acceptance line asks the preview pins for "grouped refusals"; that cell is not met as written, and the dev reported it as deviations (1) with premise_still_valid: true. Does it need the maintainer: not to land. It is a measured engine limitation, not a policy choice; the maintainer's one ruling (item 3) is honoured in full, and the business goal of the card (one confirmed call, refusals grouped, no bypass, a few hundred rows in a handful of calls) is met. What it does need is the seat's own record of the deviation in the 席位核对 and a filed follow-up card for an engine delete dry run (the dev lists it "noted, not filed"), since the card closes on Fixes with that acceptance cell open — should-fix 4.
  • Deviation (2) (group key on code + status + message, no rule name): measured in ①; the right key given what the engine attaches. Deviation (3) (the preview IS the unconfirmed call): exactly what reuse of feat(mcp)!: delete_record refuses a call without confirm: true #30861's one rule means; a separate preview: true mode would have been the second shape the order forbids. Deviation (4) (README, the surface pins, the engine-double ledger): the same three carriers feat(mcp)!: delete_record refuses a call without confirm: true #30861 reported beyond its surface; accepted on the same ground. Deviation (5) (runtime untouched): true, and the reason A3 had to mutate the bridge.
  • open_questions: none. security_findings: none from the dev; this review adds should-fix 3 (a data:write-only token reads ids and counts through the preview, under RLS), which the dev listed as out-of-scope and which is a scope-design question for the seat, not a hole.
  • File surface: all 17 files inside the claim's surface plus the three reported carriers; no packages/runtime change (the claim allowed one "if needed"); packages/spec docblock only; no ui/**, skills/**. The ledger row's comment in packages/spec/src/api/error-code-ledger.zod.ts was left alone (nit 1) — the conservative read of "beyond a docblock on the confirmation contract".
  • Overlap with PR feat(mcp): read_file and create_upload_link read and attach a record's files #31009 (card MCP cannot read or attach files: no tool downloads an attachment (e.g. a policy PDF to analyse) and none attaches one (e.g. a photographed invoice) to a record #30892, claude/project-thread-s2vcdk, head 8d23d823, open draft, another thread pushing). git merge-tree --write-tree ef1973df 8d23d823 → exit 1, three content conflicts, all count-and-prose: content/docs/ai/connect-mcp.mdx at the "Thirteen tools" sentence (vs. "Twelve tools … and two more for files") and the tools/list comment ("thirteen" vs. "twelve … fourteen with read_file and create_upload_link"); packages/mcp/src/mcp-http-tools.unknown-argument-keys.test.ts at the sweep's title and toHaveLength(13) vs. (14) (the merged truth is fifteen, the VALID_ARGS entries themselves auto-merge); packages/mcp/src/skill-md.ts at the two import lines (this PR imports DELETE_RECORDS_MAX_PER_CALL, feat(mcp): read_file and create_upload_link read and attach a record's files #31009 imports MCP_OAUTH_SCOPE_DATA_WRITE; the union is the resolution). mcp-http-tools.ts and packages/runtime/src/domains/mcp.ts auto-merge (different regions; feat(mcp): read_file and create_upload_link read and attach a record's files #31009's file tools register under their own bridge members, outside if (canWrite)'s data block). Whichever lands second takes those three hunks; the dev's out_of_scope_findings names this correctly. No semantic overlap: feat(mcp): read_file and create_upload_link read and attach a record's files #31009 adds no delete path and does not touch remove, query or aggregate.
  • Independence: rendered by an isolated contract-review subagent fed the card and its rulings, feat(mcp)!: delete_record refuses a call without confirm: true #30861's body and merge, the PR body, the net diff, the cited bridge / runtime / protocol / engine code on origin/main, the ledger and pin test, the merge-tree and the check-runs — not the seat's or the dev's conclusions; adopted by the seat named in Reviewed-by:.

④ Design judgment

  • No new execution path is the whole design, and it holds. The tool is a composition of three existing bridge members under the existing confirmation predicate. Where it could have reached for the engine (a bulk deleteMany, a transaction, a system context for the count) it did not: each row is one remove, so each row's cascade, hook verdict, restricted relation and RLS read are the engine's own, and the answer is exactly what delete_record would have said row by row, counted. The REST deleteMany route is the one precedent for a many-row delete on the platform, and the cap, the one-row-at-a-time semantics and the id-ordered walk mirror it. Declared ⇒ enforced holds: the description, SKILL line, docs and docblock promise "each under your permissions, RLS and the object's delete rules, which no caller can bypass", and the code has no member that could do otherwise. Holds.
  • The preview as the refusal. A 428 whose text content carries preview beside error keeps the error envelope identical to run_action's and delete_record's (same keys, same order, details typed ActionConfirmationRequiredDetails), so a client that already branches on the one-row refusal needs no new branch, and a client that wants the size reads one more key. The reading of the selection on an unconfirmed call is the one thing that differs from delete_record, and it is a read as the caller. Holds.
  • Partial progress is honest. remaining is defined as "of the records matched when this call began, how many still match" and counts refused, not-removed and past-cap rows alike; next tells the caller both that the same call continues and that refused rows will be attempted first again. A refused-first stall is the caller's to break with $nin, which the answer names. A bridge without aggregate answers matchedMoreThan rather than a count. Holds.
  • The pins. The unit file drives the real stdio bridge over the real HTTP transport (and once over InMemoryTransport) against a double that refuses by lock and by owner and carries details to withhold; the dogfood drives the runtime's real bridge on a booted stack with a real package hook and an administrator, plus a REST control on the door the mutation does not touch. Three ablations, each restored to blob == HEAD. Holds.

Findings (none must-fix):

  1. should-fix — packages/mcp/src/mcp-http-tools.ts, resolveDeleteSelection, the filter read limit: cap + 1. On a host that sets OS_DATA_MAX_PAGE_SIZE (or OS_DATA_QUERY_ROW_BUDGET) at or under 200, findData refuses the 201-row probe with MAX_RECORDS_EXCEEDED ("refuse, never clamp"), so every where selection — including one matching three rows — is refused, while recordIds (≤ 200) and delete_record work. Loud, never wrong, so not a must-fix; and 200 is a plausible setting since it is the REST batch cap. Fix: read limit: cap and, when the bridge carries aggregate, decide moreThanCap by whether the count exceeds the cap; keep the cap + 1 probe only for a bridge without aggregate (or document the floor on OS_DATA_MAX_PAGE_SIZE beside the tool). One function; a delta review by this reviewer suffices.
  2. should-fix — deleteEach's catch: a throw with no code (a driver fault, a lost connection) is grouped as a refusal keyed on its message, with no code, and the loop continues through the remaining ids; the answer is then isError: false with, say, refused: 150, refusals: [{ reason: "connection reset", count: 150 }], which an agent reads as the rules' verdict. A rule's refusal always carries a code (RECORD_LOCKED, PERMISSION_DENIED, the exposure codes). Fix: stop on the first uncoded throw and answer it through errorResultWithCode with the progress so far (deleted, refusals, the failed id), or at least name such a group faulted rather than refused. Not blocking: the ids are named and nothing is misreported as deleted.
  3. should-fix (seat decision, one line) — delete_records is registered under data:write alone, and its preview reads ids and counts through query / aggregate under the caller's RLS. A token granted data:write without data:read could not list ids before and now can (the dev's third out-of-scope finding). Either register the tool only when data:read is granted too (the selection is a list-class read, as the exposure gate already treats it), or state in the docs' security model that data:write implies the selection read. Not a hole: RLS applies and only ids are served.
  4. should-fix (seat, not the PR) — the card's acceptance cell "preview … grouped refusals" is not met and cannot be on this engine (③). The seat records the deviation in the 席位核对 and files the engine delete dry-run follow-up as a card (the dev's "noted, not filed") so the cell has an owner when this card closes on Fixes; the maintainer's 速读 line already says it plainly, so no ruling is needed to land.

Nits:

  • nit — packages/spec/src/api/error-code-ledger.zod.ts, the @objectstack/mcp row's comment on ACTION_CONFIRMATION_REQUIRED names only recordDeleteConfirmationRefusal and actionName: 'delete_record'; the row now also covers deleteConfirmationRefusal with actionName: 'delete_records'. Comment only; the pin test's "one second emitter" still reads true. For the seat's delta or the next spec touch.
  • nit — the SKILL.md served by the server teaches delete_records; the governed skills/objectstack-ai/SKILL.md and the Console mirror (just repaired for delete_record by fix(console): the skill download teaches delete_record confirm: true and mirrors the served SKILL.md #31015) do not. Outside this card and not owed (Metadata-protocol: none); the seat may file it beside skills: objectstack-ai SKILL.md should teach delete_record confirm: true (after #30813) #30863 / ui: Console MCP skill mirror should teach delete_record confirm: true (after #30813) #30865's pattern.
  • nit — idOfRow reads a numeric id as String(id), but the id-list read filters with $in: listed (strings); on a numeric-id table a listed id would match or not by the driver's coercion. No shipped object has numeric ids over MCP; worth one sentence when the file is next touched.
  • nit — the PR title says "deletes many records of one object" while the card body's walkthrough spans twenty objects; the card's acceptance ("a few hundred rows across objects in a handful of calls") is met with one call per object, which the docs row states. Prose only.

Implemented-by: claude/project-thread-nhupf1
Reviewed-by: session_01L9qibyvxbrLP4jm4PargoH

VERDICT: PASS

Record written 2026-11-06T18:35Z.

…th data scopes

Contract-review fixes on the delete_records tool (#30891):

- The filter selection is read at the cap, not one row past it; whether
  more match is the aggregate count's answer. The cap+1 probe stays only
  for a bridge without aggregate. The per-call cap follows the deployment's
  read window down (OS_DATA_MAX_PAGE_SIZE / OS_DATA_QUERY_ROW_BUDGET), since
  the data door refuses a larger read; an id list longer than that cap is
  refused before anything is read.
- A row's throw is judged with the REST data door's own classifier
  (mapDataError): a 4xx verdict is a refusal and is grouped, a 5xx verdict
  is a fault and stops the call, answered isError with the fault, the
  record it stopped at and the call's progress.
- delete_records registers only under both data:read and data:write.
- The error-code ledger's ACTION_CONFIRMATION_REQUIRED rows name both
  delete tools.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5
…ords bullet

#31015's drift pin compares the console's renderObjectStackSkill with the
MCP server's renderSkillMarkdown byte for byte; the server's SKILL.md now
teaches delete_records, so the console copy carries the same bullet and the
tool-name pin lists it. The server-side bullet is reflowed to the file's
line width; its words are unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5
The two new reads in the data:write-only refusal pin were of type unknown,
which grew the package's test-typecheck debt; they are cast to the
JSON-RPC shapes they read, as the rest of the file's new code should be.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-tesla · session_01L9qibyvxbrLP4jm4PargoH

Contract review · delta

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 4e5b0f2dc8e8b3e32eda6679524596fd1057b8dd (delta over the reviewed head ef1973dfe1dc58bfd1bc870f2a507a3c17a0d9d6)
Local-runs: none
Scope: the four commits over ef1973df: ad203a28 (+735 / −213 over 10 files: packages/mcp/src/mcp-http-tools.ts — deleteRecordsCap, resolveDeleteSelection rewritten around readIds, countSelection's decimal-string arm, DeleteFault / DeleteEachOutcome, rowRefusalOf, deleteEach's fault break, deleteRecordsOutcome's stoppedAt / notAttempted, deleteRecordsFaultMessage, the registration moved under if (canRead) inside if (canWrite), the id-list cap refusal before any read; mcp-delete-records.test.ts +189 (the read-window and fault suites, windowedBridge, fillRows); mcp-http-tools.scopes.test.ts +38; mcp-server-runtime.authoring-scope.test.ts; the dogfood +177 (GuardedRow, the sandboxed dlr_approved_guard, the code hook dlr_broken_store, withReadWindow, three new tests); skill-md.ts, README.md, connect-mcp.mdx, the changeset, packages/spec/src/api/error-code-ledger.zod.ts two comments); the main merge 17a02908; 47cce536 (ui/apps/console/src/pages/developer/objectstack-skill.ts +12, objectstack-skill.test.ts +1, skill-md.ts reflow, the changeset's @object-ui/console line); 4e5b0f2d (mcp-http-tools.scopes.test.ts two casts) + cited on origin/main: packages/types/src/data-error-classification.ts 275–287 (declaredHttpStatus: status then statusCode, 400–599), 144–500 (structuredCodeAnswer's arms and their statuses), 565–567 (mapDataError), 1282–1500 (classifyDataError: sandbox crash and sandbox own fault → UNCLASSIFIED_FAULT 500, structured arms with fiveXxArmDisplacesDeclared4xx, PERMISSION_DENIED 403, the innerMessage door → declared status or 400 with the undeclared hook refusal code, the declared-status passthrough), 1560–1900 (unique violation 409, the sandbox wrapper, the [EnvironmentKernelFactory] 404 / 502 / 503 limb, "Record X not found" 404, unknown column 400, not-null arm, the 500 terminal); packages/types/src/query-read-limits.ts 141–160 and 227–280 (resolveQueryReadLimits, planQueryReadWindow: refuse, never clamp); packages/mcp/src/mcp-http-tools.ts at the head, 2469–2473 (messageOf), 1922–2178 (the canRead / canWrite blocks).

Inputs read: the coordinator's delta request; the two seat claim amendments 6692914335 at 2026-11-06T18:57Z and 6692945782 at 2026-11-06T18:58Z (the two console files, text-only, under #31015's drift pin); the round-2 PR body /mnt/project-files/server-seat-2/r2026-11-06/out/30891-pr-body-r2.md (19,254 bytes, written 2026-11-06T19:43Z), diffed against the PR body GitHub serves now (GitHub still carries the round-0 body: the 速读 risk line, the "17 条", the data:write opening sentence, the cap paragraph, the "Also:" paragraph and the Files list all differ, i.e. the r2 body is on disk, not yet posted); the gates record out/30891-gates-ran-4e5b0f2d.list (124 lines, every one :: exit 0; three families more than the ef1973df list: check-sdui-manifest.mjs and its --self-test, pnpm --dir ui exec vitest run scripts/, the ui/** touch's families); the out/ directory for ablation logs: none for #30891 (only 31018-ablation-*.log exist; B1–B4 are recorded in the r2 body alone); the four commit messages and trailers; git merge-tree --write-tree of the merge commit's parents (ad203a28, 41dc3024) → tree a0032b9f, identical to 17a02908's tree, so the merge brought nothing but main; the blob of packages/mcp/src/mcp-http-tools.ts at ad203a28, at 17a02908 and at 4e5b0f2d, all d06c686d20cf; PR #31009 (card #30892, branch claude/project-thread-s2vcdk, head now 57724d86, open draft, mergeable_state: clean) and git merge-tree --write-tree pr-31024 57724d86 → tree e195c5ab, exit 1, the same three content conflicts as on ef1973df. Check-runs on 4e5b0f2d, read at 2026-11-06T19:47Z: 42 total, 14 completed/success (Check PR Size, Governed Surface Queue Guard, PR-body guards, Spec property liveness, Flag docs affected, Dogfood Regression Gate, Plan Test Core shards, Auto Label, filter and the early lint legs), 7 completed/skipped, 0 failed, 21 in_progress (Build Core, Build UI Deps, Build Docs, Check Changeset, every Type Check · lane, the remaining Lint · legs, Test Core 1–8, Live E2E UI). The head is two commits behind origin/main (90523379 #31018, 468e0c16 #30980; 84 files, none of this diff's); mergeable: true, mergeable_state: blocked (draft).

① Derived judgments

  • (a) Fault vs refusal through mapDataError: sound. rowRefusalOf asks the REST /data door's own classifier (@objectstack/types, already an import of this file) and draws the line at its status: a verdict of 500 or more is a fault (undefined, the loop breaks), anything under it a refusal to group. Read against classifyDataError's order: a sandbox crash or the sandbox's own fault → 500 (fault, right); the structured arms → DELETE_RESTRICTED / CONCURRENT_UPDATE / DUPLICATE_RECORD 409, VALIDATION_FAILED / INVALID_FIELD 400, FEEDS_DISABLED / ATTACHMENT_* / RECORD_NOT_ACCESSIBLE 403, OBJECT_NOT_FOUND 404, OBJECT_API_* 404 / 405 (all refusals, grouped, right), ERR_DATASOURCE_UNAVAILABLE 503 (fault, right), and fiveXxArmDisplacesDeclared4xx keeps a producer-declared 4xx over a 5xx arm (right: a rule's verdict wins); PERMISSION_DENIED 403; a sandboxed body's throw with innerMessage → its declared status or 400 with the undeclared hook refusal code HOOK_ABORTED (grouped, right; pinned in the unit file with the SandboxError shape and in the dogfood with a real body hook and a REST control answering 400 HOOK_ABORTED); the declared-status passthrough honours status or statusCode in 400–599, so RECORD_LOCKED / 409 and a package's 422 group and a declared 502 / 503 stop; a driver's string code with no status (ECONNREFUSED, a SQL state, SQLITE_BUSY) matches no arm, has no innerMessage and no declared status, so it reaches the text heuristics and the 500 terminal (fault, right; pinned for ECONNREFUSED, a coded 5xx and a bare Error, and in the dogfood with a code hook's bare throw against a REST control answering 500). No 5xx is grouped: the status test comes before anything else in rowRefusalOf. Refusals mis-stopped: one shape only, by the door's own rule — a code hook that throws a registered code with NO status ({ code: 'RECORD_LOCKED' } alone) matches no structured arm and falls to the 500 terminal, so the walk stops at it; REST answers that producer 500 too, HotCRM's lock declares 409, and the frame's rule is door parity, so this is the door's verdict, not the tool's (nit 1). 4xx verdicts that are store conditions (a driver's "unknown column" → 400 INVALID_FIELD, a [EnvironmentKernelFactory] … not found → 404) would be grouped and the walk would go on, bounded by the cap; again the door's own reading (nit 2). One sentence of the dev's claim is narrower than written: "the two doors cannot disagree on a row" is true of the STATUS verdict; for a coded sandboxed refusal the reason is thrownRefusalEnvelope(err).message, i.e. messageOf(err), the [sandbox] … threw: wrapper, where REST serves innerMessage — the same spelling delete_record has served since feat(mcp)!: delete_record refuses a call without confirm: true #30861, so parity with the one-row tool holds and the follow-up is shared (should-fix 2). TRUE.
  • The fault answer is honest about what happened. deleteEach counts attempted up to and including the faulted row, breaks, and reports notAttempted = ids.length − attempted; the handler renders isError: true, an error carrying the fault's own code and status when declared and a message naming the record, the deleted count ("stay deleted") and the not-attempted count, plus progress (the full outcome with stoppedAt, notAttempted, remaining = matched − deleted, and no next). Pinned: t1 deleted, t2 refused, t3 the fault, t4 / t5 never asked (engine.delete called 3 times), and in the dogfood by REST ground truth (two 404s, the approved row 200, the broken row 200, the row past the fault 200). TRUE.
  • (b) The cap follows the read window down: right, and not scope creep. The round-1 finding was that limit: cap + 1 is refused on a host whose window is 200; reading at the cap fixes exactly 200 and leaves every smaller window refusing every filter selection with a MAX_RECORDS_EXCEEDED that names a limit the caller never sent. deleteRecordsCap() = min(200, OS_DATA_MAX_PAGE_SIZE, OS_DATA_QUERY_ROW_BUDGET) through the platform's own resolveQueryReadLimits (the protocol reads the same variables per request); the read is then at most the window, the answer's cap, wouldAttempt, remaining and next all speak the real cap, and an id list longer than it is refused before any read (so the window's refusal never surfaces with the wrong noun). It is one exported function, in the tool's file, on a dependency the file already had, with no new surface beyond the answer's cap taking a smaller value; the alternative (document a floor on the variable) would leave the tool unusable on such hosts. rowBudget is "offset plus rows" and the tool reads at offset 0, so the min is exact. Pinned: unit ×6 (page size = cap with a lit control refusing 201 through planQueryReadWindow in front of the real bridge; exactly the cap with no next; a decimal-string count; page size 50 and row budget 50 lowering the cap with cap of 50 in next; an id list over a lowered cap refused before find), dogfood ×2 (page size 200 with a REST ?limit=201 → 400 MAX_RECORDS_EXCEEDED control; page size 2 running at 2 and refusing a 3-id list). Two residues, neither wrong: the stdio transport reads the engine directly and applies no window, so a lowered cap there is unneeded but consistent (nit 3); the probe cap + 1 survives only for a bridge without aggregate (or a count it cannot read), the one way to tell "exactly the cap" from "more", and on a window of exactly the cap that bridge would still be refused — the docblock says so. TRUE.
  • The count race is handled. With aggregate, a full read of cap rows is followed by the count; matched = max(count, ids.length) so a write landing between the two reads cannot report fewer matched than attempted; moreThanCap = matched greater than cap. Without a count the probe decides. TRUE.
  • (c) Nothing was lost in the replay. The blob B1 mutated, d06c686d20cf, is the blob at ad203a28, at the merge 17a02908 and at the head 4e5b0f2d (three equal ids), and the merge commit's tree equals a clean merge-tree of its two parents, so the merge changed nothing of the branch's own. Each of sf1–sf3 and the ledger nit is present in the head file by line: deleteRecordsCap (646), readIds(cap) under the aggregate branch (745) with the cap + 1 probe only after it (755), the id-list cap refusal before any read (2125–2130), rowRefusalOf with mapDataError and the 500 test (828–830), the fault break and notAttempted (882–902), stoppedAt on the outcome (967), the progress render (2167), the registration under if (canRead) inside if (canWrite) (closing at 2178), and the two ledger comments naming both tools and deleteConfirmationRefusal. B2–B4's anchors (door.status, readIds(cap), deleteRecordsCap) are lines of that blob, and their restores report blob == HEAD (d06c686d20cf). TRUE.
  • (sf3) Both data scopes. canRead = !scopes || data:read granted, canWrite likewise; the tool registers only inside both, so an API-key or session principal (no scopes) keeps it, data:write alone does not list it and a tools/call under that grant is refused before query, aggregate or remove (pinned with the bridge's call log empty and delete_record still callable as the control; 4e5b0f2d only types the two answers, no assertion changed). README, docs row, docs bullet, SKILL line, the changeset's Clause-② sentence and the console mirror all say both scopes. Ablated (B1: if (canRead) → if (true), 3 red across the scope and authoring-scope pins, "expected [ [ 'query', … ] ] to deeply equal []"). TRUE.
  • The console mirror. objectstack-skill.ts gets the one bullet, with the same words as skill-md.ts after the reflow (the server bullet interpolates ${DELETE_RECORDS_MAX_PER_CALL}, the console copy spells 200; fix(console): the skill download teaches delete_record confirm: true and mirrors the served SKILL.md #31015's drift pin compares the rendered texts); the tool-name pin lists delete_records; '@object-ui/console': patch is in the one changeset; the two files are the two the amendments name, text only, nothing else under ui/**; the gates list grew by the ui families and all exit 0; the r2 body cites both amendment ids. TRUE.
  • Tests claimed vs. on the head. The unit file's it / it.each lines expand to 17 + 6 (read window) + 4 (faults) = 27, as the r2 body says; the dogfood has 7; the scopes pin 11. TRUE.
  • Ablations B1–B4. Each mutates one line of the head blob, each names its red tests and one assertion line, each restores to blob == HEAD; B2 and B3 are red in the dogfood as well with the dist rebuilt inside the mutation and after the restore. B1's pre-commit timing and the wiped working tree are disclosed and repaired by the byte-identity proof. The outputs live only in the r2 PR body: no out/30891-ablation-*.log on disk, unlike fix(runtime)!: the package enable door refuses while a rule it would serve reads a field a disabled package withholds #31018's (should-fix 1). The dev's readings, not re-run here.
  • The r2 PR body against the code. Request shape (where xor recordIds 1..200, optional boolean confirm), the preview envelope, the outcome shape with refusals[], remaining, cap, next, the fault answer (error + progress with stoppedAt, notAttempted), the 200 cap and its origin, "read at the cap, never one past it", the window paragraph, the fault paragraph, the scopes sentence, the Files list (the two ui files under 6692914335 and 6692945782, the spec doc-comment-only line naming the ledger), Clause-②: yes (widening) and Metadata-protocol: none line-initial, three deviations stated as deviations in Acceptance notes, Fixes #30891, the 速读 six lines with the updated risk and verify lines, the feat(mcp): read_file and create_upload_link read and attach a record's files #31009 serial note (three hunks, merged count 15, the console mirror duty) — every line has a line in the diff. Not yet on GitHub (above). TRUE.
  • Commit trailers. ad203a28, 47cce536, 4e5b0f2d end with Co-Authored-By: Claude (the frame's prescribed spelling) and Claude-Session: …session_01L9qibx1NhZMrqCvW7FBxg5; no model name, no Fixes in any commit; the merge commit carries the default message and no trailers, as a merge does. TRUE.

② Semver level

Unchanged from round 0 in kind: Clause-②: yes (widening) and Metadata-protocol: none, line-initial, once each, in the changeset and the r2 body. The changeset now bumps three packages at patch (@objectstack/mcp, @objectstack/spec doc comments only, @object-ui/console the mirror text), which none grades; no !, nothing narrows (a tool that round 0 registered under data:write alone and this head registers under both scopes is a narrowing of an unreleased surface, inside the same changeset). The ui/** touch is the seat's leave on record (two amendments), text only, no governed path; 1,236 + 758 + 23 + 8 changed lines on the branch's own commits, under the 3,000 line threshold.

Check-runs on 4e5b0f2d at 2026-11-06T19:47Z: 14 success / 7 skipped / 0 failed / 21 in progress (every build, type-check and Test Core lane still running). The seat re-reads before landing.

③ Boundary flags

④ Design judgment

Findings (none must-fix):

  1. should-fix (evidence, seat) — the B1–B4 ablation outputs exist only inside the r2 PR body; out/ holds no 30891-ablation-*.log where fix(runtime)!: the package enable door refuses while a rule it would serve reads a field a disabled package withholds #31018's round left one per ablation. Save the four runs' outputs (anchor, replacement, the red list, the assertion line, the restore line) as files beside the gates list, so the record does not depend on a PR body that can be edited. No code.
  2. should-fix (follow-up, both delete tools) — rowRefusalOf takes reason from thrownRefusalEnvelope(err).message whenever a code is declared, and messageOf reads .message; for a sandboxed body that throws WITH a code ({ code: 'RECORD_LOCKED', status: 409 } from QuickJS) that is the runner's [sandbox] hook body threw: … wrapper, where REST's innerMessage door serves the body's own sentence. delete_record has served the wrapper since feat(mcp)!: delete_record refuses a call without confirm: true #30861 through the same reader, so this head is at parity with the one-row tool and the dev's "cannot disagree" holds for the status. Fix, in one place: let thrownRefusalEnvelope (or messageOf) prefer a non-empty string innerMessage, with a pin on both tools; a small follow-up card for the MCP tools, not this PR.

Nits:

  • nit — a code hook that throws a registered code with no status or statusCode ({ code: 'RECORD_LOCKED' } alone) reaches the classifier's 500 terminal and stops the bulk walk at that row, exactly as REST answers it 500; HotCRM's rules declare their status. Worth one sentence in content/docs/automation/hooks.mdx when it is next touched: a rule's refusal declares a 4xx status.
  • nit — two of the door's 4xx text heuristics (a driver's "unknown column" → INVALID_FIELD 400; a [EnvironmentKernelFactory] … not found without a provisioning or failed marker → 404) would group a store condition as a refusal and keep walking, bounded by the cap. The door's own reading, inherited on purpose; noted so the seat knows the shape.
  • nit — the stdio transport reads the engine directly and never meets the read window, so deleteRecordsCap() lowers its cap on an OS_DATA_*-configured host without need; consistent across transports and harmless.
  • nit — deleteRecordsCap trusts resolveQueryReadLimits for a positive value; the resolver's own validation (readLimitVar) was not read here. A zero or negative variable is the resolver's boot fault by the protocol's constructor comment, so nothing to change.
  • nit — the r2 body's "the SKILL.md tool list … name the tool" paragraph says @objectstack/spec changes "only doc comments", which is true, and the Files list says the same; the scripts/engine-double-contract.pinned.json row count on this head is unchanged from round 0 (the new engine double is the same file). Prose only.

Implemented-by: claude/project-thread-nhupf1
Reviewed-by: session_01L9qibyvxbrLP4jm4PargoH

VERDICT: PASS (delta)

Landing waits on: CI finishing green on 4e5b0f2d (21 lanes in progress at the last read, 0 failed), the seat's relay write of the r2 body to the PR (GitHub still serves round 0), and the seat's call on the two-commit origin/main gap; the #31009 three-hunk reconciliation and the console mirror fall to whichever lands second.

Record written 2026-11-06T19:50Z.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xl tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP: delete records by filter in one confirmed call, with refusals grouped by reason, instead of one delete_record per row

1 participant